fix(firestore): avoid reusing busy clients for streams#7950
fix(firestore): avoid reusing busy clients for streams#7950akshatbaranwal wants to merge 2 commits intogoogleapis:mainfrom
Conversation
|
@akshatbaranwal Does this change solve any open issues with this SDK? |
|
Yes — this is the Firestore-side mitigation for #7960. That issue describes how a single stalled REST-fallback server-streaming read ( This PR doesn't try to fix the underlying transport stall (that's tracked separately in #7959). It's a defensive change in Happy to scope it differently (e.g. stale-stream eviction instead of an opt-in reuse policy) if that's closer to what you'd prefer — question #1 in #7960 was explicitly asking for maintainer guidance on that. |
Summary
preferIdleClientspath inClientPoolrequestStream()so new streams reuse idle clients but do not pile onto already-busy onesWhy
DocumentReference.get()andgetAll()use thebatchGetDocumentsserver-streaming path. If one stream stalls for a long time, the existingClientPoolstrategy prefers the most-full client and can keep routing later low-concurrency reads back onto the same busy client.This change is a defensive mitigation: request streams will reuse idle clients, but if all existing clients are already busy, they open a fresh client instead of piling onto a busy one.
Testing
cd handwritten/firestore && npx mocha -r ts-node/register/transpile-only dev/test/pool.ts --grep "preferIdleClients|re-uses idle instances when preferIdleClients|creates a new client when preferIdleClients|creates new instances as needed|re-uses idle instances"cd handwritten/firestore && npx tsc -p . --noEmit --skipLibCheck